feat: make prompt and librarian client timeouts configurable (#874) - #1108
Open
sunnyadn wants to merge 1 commit into
Open
feat: make prompt and librarian client timeouts configurable (#874)#1108sunnyadn wants to merge 1 commit into
sunnyadn wants to merge 1 commit into
Conversation
Second slice of trustgraph-ai#874. A spec that creates a client resolves its timeout as: explicit constructor value, else the processor attribute the spec names, else the class default (TimeoutSpec mixin). RequestResponseSpec and LibrarianSpec thread the result into the client they create. PromptClient and AsyncLibrarianClient methods default to None and fall back to it, per-call timeouts still override. --prompt-timeout (600) on FlowProcessor. --librarian-timeout (120) via WorkspaceProcessor.add_librarian_args, called by FlowProcessor and the cores service, which also passes it to its direct AsyncLibrarianClient. 1 default moves: a bare prompt-request wrapper call with no timeout goes from the generic wrapper's 300 to the prompt default 600 (4 in-tree sites in nlp_query and structured_diag). ConfigClientSpec now follows --config-timeout with the same 60 it resolved to before.
Contributor License Agreement ✅All contributors have signed the CLA. Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second slice of #874, after #1063. Against
release/v2.9, happy to retarget.Changes
RequestResponseSpec,LibrarianSpec) now take atimeout. If none is given they read it off the processor (prompt_timeout,librarian_timeout,config_timeout) and fall back to the old literal. The impl wrapper and the client methods passNonethrough instead of pinning a number, so per-call timeouts still override.--prompt-timeout(600) onFlowProcessor.--librarian-timeout(120) viaWorkspaceProcessor.add_librarian_args, used byFlowProcessorand the cores service, so the librarian and flow services don't advertise a flag they don't use.--help, whether or not it has that client. If you'd rather havetimeout=self.prompt_timeoutat each of the ~20 call sites instead, I'm happy to do that here.Behaviour
1 default moves. A bare
flow("prompt-request").request(req)used to get the generic wrapper's 300 and now gets 600, the default every other prompt call already had. 4 callers in tree:nlp_query/service.py:170,235andstructured_diag/service.py:382,487. Keeping 300 there would have left them outside the knob, so I think 600 is the right side of this, but it is a change.ConfigClientSpec(exported, unused in tree) now follows--config-timeout, effective default still 60.document_ragkeeps--fetch-chunk-timeoutfor the chunk fetch, only its help text changed.Tests
24 new tests, 20 fail with the source reverted to
release/v2.9. Unit, contract and integration pass in CI. Also ran it against a Pulsar standalone:--prompt-timeout 2and--librarian-timeout 2cut a 5 s stub at 2 s, and the defaults behave as before.